Skip to content

feat(particle): support custom particle shaders with custom data#3004

Open
hhhhkrx wants to merge 20 commits into
galacean:dev/2.0from
hhhhkrx:feat/particle-custom-fragment
Open

feat(particle): support custom particle shaders with custom data#3004
hhhhkrx wants to merge 20 commits into
galacean:dev/2.0from
hhhhkrx:feat/particle-custom-fragment

Conversation

@hhhhkrx
Copy link
Copy Markdown
Contributor

@hhhhkrx hhhhkrx commented May 13, 2026

Summary

Allow user-authored particle shaders to override vert / frag while reusing the engine's particle simulation, and add a CustomDataModule for feeding per-particle business data into those shaders.

What's in

  • ParticleMaterial now accepts an optional Shader in its constructor, so users can pass a shader built via Shader.create(...) while keeping the existing default behavior.
  • Effect/Particle.shader restructured to match the PBR.shader pattern: the .shader file is the user-facing surface that declares an inline vert / frag, calling helpers brought in by the include. The include ShaderLibrary/Particle/ParticleVert.glsl carries:
    • All engine uniforms / Attributes / Varyings structs / 8 particle-module includes
    • Reusable helpers: computeParticleCenter, computeParticleColor, computeParticleVaryingUV
  • CustomDataModule: two per-particle vec4 streams (data0, data1). Each stream's 4 components share one mode (Constant / TwoConstants / Curve / TwoCurves). Per-particle random factors for the random modes come from hashing a_DirectionTime.w — no vertex-buffer changes needed.
  • ShaderLibrary/Particle/Module/CustomData.glsl exposes sampleParticleCustomData0 / sampleParticleCustomData1 helpers; uniforms (renderer_CustomData0MaxConst, etc.) are also readable directly by name from a user shader.
  • E2E case particleRenderer-customShader: builds a custom particle shader at runtime that reads both renderer_CustomData0MaxConst (color tint) and renderer_CustomData1MaxConst (position offset) directly. Verifies the TS-side customData.dataN.x.constantMax = ..._updateShaderData → GPU uniform → shader read round-trip end to end.

How a user customizes a particle shader

Copy Effect/Particle.shader as a starter, edit the inline vert / frag:

Varyings vert(Attributes attr) {
    Varyings v;
    float age = renderer_CurrentTime - attr.a_DirectionTime.w;
    float normalizedAge = age / attr.a_ShapePositionStartLifeTime.w;
    if (normalizedAge >= 0.0 && normalizedAge < 1.0) {
        vec3 center = computeParticleCenter(attr, age, normalizedAge, v);
        center.y += sin(normalizedAge * 6.2831853) * 0.5;          // custom motion
        gl_Position = camera_ProjMat * camera_ViewMat * vec4(center, 1.0);
        v.v_Color = computeParticleColor(attr, attr.a_StartColor, normalizedAge);
        v.v_Color.rgb *= sampleParticleCustomData0(attr, normalizedAge).rgb;  // custom tint
    } else {
        gl_Position = vec4(2.0, 2.0, 2.0, 1.0);
    }
    return v;
}

Test plan

  • tsc passes for packages/core and e2e
  • npm run precompile produces all 22 shaders cleanly with the new include layout
  • tests/src/core/particle/ — 77/77 unit tests pass
  • E2E Particle.customShader runs and produces the expected orange-tinted, right-shifted particles
  • Visual regression on remaining Particle.* e2e cases (run on CI)

Summary by CodeRabbit

  • New Features

    • Per-particle custom data streams (two vec4 channels) for richer particle effects and custom shaders.
    • Particle materials can use a custom shader to control particle rendering.
  • Public API

    • Exposed custom-data module/stream and two new random-seed constants; particle generators now include custom data support.
    • Material cloning preserves custom shader selection.
  • Tests

    • Added unit and E2E tests validating custom-data behavior and custom-shader particle rendering.

Review Change Stack

- ParticleMaterial accepts an optional user-built Shader
- Split Effect/Particle.shader into a thin top-level shader and a
  ShaderLibrary/Particle/ParticleVert.glsl include exposing helpers:
  computeParticleCenter, computeParticleColor, computeParticleVaryingUV
- Add CustomDataModule with two per-particle vec4 streams; modes:
  Constant / TwoConstants / Curve / TwoCurves (per-particle random
  factors derived from birth-time hash)
- Add ShaderLibrary/Particle/Module/CustomData.glsl exposing
  sampleParticleCustomData0 / sampleParticleCustomData1 helpers
- Add e2e case verifying TS -> uniform -> shader round-trip
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds CustomDataModule exposing two per-particle vec4 streams (data0, data1); wires it into ParticleGenerator and shaders; updates shader library and compiled metadata; adds unit tests and an E2E custom-shader test with config.

Changes

Particle Custom Data Streams

Layer / File(s) Summary
Data structures and public API
packages/core/src/particle/modules/CustomDataModule.ts, packages/core/src/particle/enums/ParticleRandomSubSeeds.ts, packages/core/src/particle/ParticleMaterial.ts, packages/core/src/particle/index.ts
Adds CustomDataStream, new random sub-seed constants, makes ParticleMaterial accept an optional shader parameter, and re-exports the new types.
ParticleGenerator wiring
packages/core/src/particle/ParticleGenerator.ts
Adds readonly customData: CustomDataModule, initializes it in the constructor, and invokes its _updateShaderData and _resetRandomSeed at the appropriate lifecycle points.
CustomDataModule implementation
packages/core/src/particle/modules/CustomDataModule.ts
Implements CustomDataModule with data0/data1 streams, shader macro/property bindings, per-stream upload logic for constant or curve modes, component-mode validation, and per-stream random seed handling.
Shader library & compiled shader
packages/shader/src/ShaderLibrary/index.ts, packages/shader/compiledShaders/Effect/Particle.shaderc
Registers Particle_Module_CustomData and Particle_ParticleVert in shader library and regenerates Effect/Particle.shaderc instruction metadata.
E2E test and config
e2e/case/particleRenderer-customShader.ts, e2e/config.ts
Adds an E2E test that compiles an inline custom shader using renderer_CustomData* inputs, configures generator constants for tint/offset, runs the simulation, captures a screenshot, and registers the test in config.
Unit tests
tests/src/core/particle/CustomData.test.ts
Adds vitest coverage validating default state, enable/disable, constant/two-constants/curve/two-curves modes, mixed-mode validation, seed reset, and runtime update behavior.

Sequence Diagram

sequenceDiagram
  participant Test as E2E Test
  participant Engine as Engine
  participant Material as ParticleMaterial
  participant Generator as ParticleGenerator
  participant CustomData as CustomDataModule
  participant ShaderData as ShaderData
  Test->>Engine: create engine with ShaderCompiler
  Test->>Material: new ParticleMaterial(engine, compiledCustomShader)
  Test->>Generator: configure generator and enable customData streams
  Test->>Generator: set customData constants for tint and offset
  Generator->>CustomData: _updateShaderData(shaderData)
  activate CustomData
  CustomData->>ShaderData: upload data0 constants/gradients
  CustomData->>ShaderData: upload data1 constants/gradients
  CustomData->>ShaderData: enable mode macros
  deactivate CustomData
  Test->>Engine: updateForE2E (simulate frames)
  Engine->>Material: render particles with custom shader
  Test->>Test: initScreenshot (capture output)
Loading

🎯 3 (Moderate) | ⏱️ ~25 minutes

🐰 I stitched two vec4 streams so bright,
Constants and curves leap into light,
Shaders sip colors, offsets take flight,
Particles shimmer through the night.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding support for custom particle shaders with a custom data module for per-particle data.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

❌ Patch coverage is 81.75676% with 54 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.98%. Comparing base (0d29d9a) to head (b9a8b90).

Files with missing lines Patch % Lines
e2e/case/particleRenderer-customShader.ts 0.00% 40 Missing and 1 partial ⚠️
e2e/config.ts 0.00% 6 Missing ⚠️
...ages/core/src/particle/modules/CustomDataModule.ts 97.46% 6 Missing ⚠️
packages/core/src/particle/ParticleMaterial.ts 75.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           dev/2.0    #3004      +/-   ##
===========================================
- Coverage    78.43%   77.98%   -0.46%     
===========================================
  Files          903      905       +2     
  Lines        99951   100172     +221     
  Branches     10314    10345      +31     
===========================================
- Hits         78400    78117     -283     
- Misses       21376    21877     +501     
- Partials       175      178       +3     
Flag Coverage Δ
unittests 77.98% <81.75%> (-0.46%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

CI runner showed 1.296% diff against a baseline captured locally —
expected platform-driven AA / float-precision variance for an
untextured large-quad particle case. Bumping diffPercentage to 1.5
with the same headroom precedent as particleFire / horizontalBillboard.
@hhhhkrx hhhhkrx self-assigned this May 13, 2026
GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

hhhhkrx added 2 commits May 18, 2026 16:48
…om-fragment

# Conflicts:
#	packages/shader/compiledShaders/Effect/Particle.shaderc
Covers CustomDataStream defaults, enabled toggle, constant/two-constants/
curve/two-curves upload paths, mixed-mode error, random-seed reset and
engine-update integration. Raises codecov patch coverage for the new
custom data module above target. Also regenerates Particle.shaderc after
the dev/2.0 merge.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
tests/src/core/particle/CustomData.test.ts (1)

86-150: ⚡ Quick win

Strengthen assertions: current tests verify no-throw, not data correctness.

These mode tests pass even if uniforms are wrong but no exception is thrown. Assert uploaded shader values/macros for each mode to catch functional regressions in _updateShaderData.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/src/core/particle/CustomData.test.ts` around lines 86 - 150, Tests
currently only assert _updateShaderData doesn't throw; strengthen them to verify
particleRenderer.shaderData actually received the correct macros and uniform
values for each mode. After calling
customData._updateShaderData(particleRenderer.shaderData) in each it block,
assert that shaderData reflects customData.enabled and the proper CUSTOMDATA
mode flag (reference: customData.enabled and _updateShaderData), then check the
uploaded uniform arrays/values for data0 and data1 match the expected constants
or curve samples derived from the ParticleCompositeCurve/ParticleCurve/CurveKey
inputs (use the ParticleCompositeCurve instances you construct to compute
expected numbers for constant, two-constant ranges, curve samples, and min/max
samples for two-curves). Use particleRenderer.shaderData (the same object passed
in) to read back macro flags and uniform buffers and assert equality to catch
regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/src/core/particle/CustomData.test.ts`:
- Around line 23-40: The test's beforeAll creates and runs a real WebGLEngine
but never tears it down; add an afterAll that stops and destroys the engine to
prevent resource leakage by calling engine.run(false)/engine.stop if applicable
and await engine.destroy() (or engine.destroy?.() if optional), then
null/undefined the engine reference and any created canvas; target the engine
variable created in beforeAll and the WebGLEngine.create usage so the cleanup
runs after the suite.
- Around line 193-199: Save the original performance.now and any engine-internal
fields you mutate before the test, then restore them after the loop (preferably
in a finally block or test afterEach). Specifically, capture the original
performance.now reference, and any engine properties changed around where
engine.update() is called (e.g., the engine timing/state fields you modified),
run the performance.now override and the for-loop of engine.update(), and then
restore performance.now and those saved engine fields to their originals so
later tests aren’t affected.

---

Nitpick comments:
In `@tests/src/core/particle/CustomData.test.ts`:
- Around line 86-150: Tests currently only assert _updateShaderData doesn't
throw; strengthen them to verify particleRenderer.shaderData actually received
the correct macros and uniform values for each mode. After calling
customData._updateShaderData(particleRenderer.shaderData) in each it block,
assert that shaderData reflects customData.enabled and the proper CUSTOMDATA
mode flag (reference: customData.enabled and _updateShaderData), then check the
uploaded uniform arrays/values for data0 and data1 match the expected constants
or curve samples derived from the ParticleCompositeCurve/ParticleCurve/CurveKey
inputs (use the ParticleCompositeCurve instances you construct to compute
expected numbers for constant, two-constant ranges, curve samples, and min/max
samples for two-curves). Use particleRenderer.shaderData (the same object passed
in) to read back macro flags and uniform buffers and assert equality to catch
regressions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 374961bc-6381-4b7d-bf93-4b4d2bb2e938

📥 Commits

Reviewing files that changed from the base of the PR and between ada8712 and 4e2f916.

📒 Files selected for processing (2)
  • packages/shader/compiledShaders/Effect/Particle.shaderc
  • tests/src/core/particle/CustomData.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/shader/compiledShaders/Effect/Particle.shaderc

Comment thread tests/src/core/particle/CustomData.test.ts
Comment thread tests/src/core/particle/CustomData.test.ts Outdated
GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

…onfig

`rootEntity.createChild()` invokes `ParticleRenderer._onEnable` synchronously,
which calls `generator.play()` while `useAutoRandomSeed` is still its default
`true`. The `Math.random()` seed picked then sticks for the rest of the run
and defeats deterministic screenshot capture, forcing a 1.5% diff headroom.

Switch to the detached-entity pattern (cf. `particleRenderer-emissive`):
create with `new Entity(...)`, configure the generator, then `addChild`. The
lifecycle hook now fires after `useAutoRandomSeed = false` is in place, so
the run is reproducible and `diffPercentage` can drop to 0 — CI will report
the residual platform variance and we tighten the threshold to match.
GuoLei1990

This comment was marked as outdated.

…d value

CI on ubuntu reports a deterministic 1.31895833333% visual regression
against the baseline (captured locally) across all 3 retries — the residual
is platform AA / float-precision drift, not nondeterminism, now that
useAutoRandomSeed is in effect. Set the threshold to 1.3191 (CI value
+ 0.0001 margin), which is meaningfully tighter than the prior 1.5
headroom and will catch any real regression beyond platform variance.
GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

…istic run

The original baseline was captured when `useAutoRandomSeed = true` was
still in effect — `Math.random()` had picked some non-reproducible seed
at capture time. After the lifecycle fix the generator runs from the
deterministic default seed `0`, so the particle layout is now stable
but unrelated to the prior baseline (≈1.32% pixel delta — not platform
AA, an entirely different particle distribution).

Replace the baseline with the CI-captured frame from commit b451e80
so the truth source matches the deterministic run, then drop
diffPercentage back to 0. Same headroom (and same enforcement strength)
as other simple particle cases like particleEmissive / shapeTransform.
GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

hhhhkrx added 2 commits May 28, 2026 14:13
removeCurve / removeGradient previously only dropped the TS-side
registry entries; ShaderData._propertyValueMap retained the last
uploaded value indefinitely. If a custom shader still referenced the
matching renderer_<name>... uniform (e.g. the user hadn't updated
shader source yet), it kept reading the stale value instead of 0 —
silent ghost data, no compile-time or runtime trace.

The remove* JSDoc already promised "Shader uniforms read 0 once
unregistered". Implement that contract: write 0 / zero-color into
shaderData via the cached ShaderProperty handles before discarding
the meta entry. Reuse module-level zero buffers (Float32Array(8) for
the vec2[4] gradient slots, Vector4() for the vec4 const slots) so
the hot path doesn't allocate.

Regression test exercises addCurve + addGradient + _updateShaderData,
then removeCurve + removeGradient, and asserts shaderData.getFloat /
getVector4 read 0 — verified by rolling the zero-out back and
confirming the test fails on a stale 0.8.
…om-fragment

# Conflicts:
#	e2e/config.ts
#	packages/shader/compiledShaders/Effect/Particle.shaderc
GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

Single-line `/** ... */` with no @param drifts from the codebase
convention (EmissionModule.removeBurst / removeBurstByIndex / etc. all
use multi-line block + @param even when the parameter is self-evident).
Expand to match.
GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

…onstructor

Match the depth of addCurve / addGradient (already complete) for the
rest of CustomDataModule's public surface:

- class: expand the one-line summary with what the module actually
  exposes — named scalar / color channels readable via
  `renderer_<name>...` uniforms.
- `curves` / `gradients` getters: add @returns clarifying the readonly
  shape.
- constructor: add @param `generator`.
Copy link
Copy Markdown
Member

@GuoLei1990 GuoLei1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查(2026-05-28,合并修正版)

确认 e3607d54 已修复 removeCurve / removeGradient 的 TSDoc(多行块 + @param + {@link} 引用,与 EmissionModule.removeBurst 风格对齐)。

撤回上一轮 TSDoc 全量 review 的过度严格判断,把所有剩余真实问题合并成一条精准 review。


撤回的过度严格判断

上轮 review 项 撤回理由
class CustomDataModule 主描述"Custom data module." 太简 → 要扩展 撤回EmissionModule 也是"The EmissionModule of a Particle Generator." 同等简短,是引擎惯例
constructor(generator) 无 TSDoc → 要补 撤回NoiseModule / MainModule / SizeOverLifetimeModule 等同类 constructor 全部无 TSDoc,是引擎惯例
get curves / get gradients@returns 撤回 — Galacean 现有 getter(如 EmissionModule.get shape() / .get bursts()) 普遍不写 @returns

致歉,这几条是"按通用 TSDoc 教条而非项目惯例" review。


真实剩余问题

[P3] get curves / get gradients 单行 TSDoc 应展开为多行块

EmissionModule.get shape() 风格对齐:

// 当前
/** Registered scalar streams keyed by name. */
get curves(): Readonly<Record<string, ParticleCompositeCurve>> { ... }

// 改为
/**
 * Registered scalar streams keyed by name.
 */
get curves(): Readonly<Record<string, ParticleCompositeCurve>> { ... }

get gradients 同理。


[P3] STREAM_NAME_PATTERN 应改为类内 private static readonly

位置:CustomDataModule.ts:13(模块级 const)

const STREAM_NAME_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/;

引擎所有粒子模块的常量都用类内 static readonly(NoiseModule 13 个、ColorOverLifetimeModule 7 个、MainModule 8 个 ... 全部如此)。CustomDataModule 自身的 _zeroCurveArray / _zeroColor 也是类内 static readonly,只有这一条用了模块级 const,风格分裂。

grep -rn "^const " packages/core/src/particle/ 全 particle 目录只有 2 处模块级 const(另一处是 FEEDBACK_SHADER_NAME),其余全部走类成员。

修复:移入类内 + 改为引擎命名风格(_streamNamePattern,camelCase + leading underscore,与 _zeroCurveArray 一致):

export class CustomDataModule extends ParticleGeneratorModule {
  private static readonly _streamNamePattern = /^[A-Za-z_][A-Za-z0-9_]*$/;
  private static readonly _zeroCurveArray = new Float32Array(8);
  private static readonly _zeroColor = new Vector4();

  // ...

  private _validateName(name: string, method: string): boolean {
    if (!CustomDataModule._streamNamePattern.test(name)) { ... }
    // ...
  }
}

[P3] STREAM_NAME_PATTERN 字符集过严 — 数字开头被误拒

位置:同上,/^[A-Za-z_][A-Za-z0-9_]*$/

引擎拼接 uniform 名时已加前缀 renderer_:

ShaderProperty.getByName(`renderer_${name}MaxConst`)

无论 name 首字符是字母 / 数字 / 下划线,拼装后 uniform 名首字符都是 r——已经满足 GLSL identifier"字母 / 下划线开头"的约束。name 本身不需要约束首字符

当前正则误拒了一批合法用法:

输入 拼接结果 当前正则 实际合法性
"0intensity" renderer_0intensityMaxConst ❌ 拒绝 合法(r 开头)
"123" renderer_123MaxConst ❌ 拒绝 合法
"42abc" renderer_42abcMaxConst ❌ 拒绝 合法
"vec3" renderer_vec3MaxConst ✅ 通过 ✅ 合法(整词非 GLSL 关键字)
"my-curve" renderer_my-curveMaxConst ❌ 拒绝 ❌ 非法(连字符)正确拒
"my space" 含空格 ❌ 拒绝 ❌ 非法,正确拒
"强度" 含非 ASCII ❌ 拒绝 ❌ WebGL 严格 ASCII,正确拒

修正后的正则:

private static readonly _streamNamePattern = /^[A-Za-z0-9_]+$/;
//                                              ^^^^^^^^^^^^ ASCII alphanumeric + 下划线
//                                                         ^ 一个或多个(避免空字符串)

放开:

  • ✅ 数字开头("0intensity")
  • ✅ 纯数字("42")
  • ✅ 用户用版本号 / 编号命名 stream 的常见场景

仍拒(与原正则一致):

  • ❌ 空字符串
  • ❌ 任何非 ASCII alphanumeric / 下划线字符(空格、标点、中文、emoji 等)

总结

级别 问题 改动量
P1 ghost data ✅ 已修(654e458)
Minor removeCurve/removeGradient TSDoc ✅ 已修(e3607d54)
P3 get curves/get gradients 单行 → 多行块 4 行(2 个 getter,各加 2 行)
P3 STREAM_NAME_PATTERN 应移入类内 + 改命名风格 3 行(移位 + 调用点加类前缀)
P3 STREAM_NAME_PATTERN 字符集过严(误拒数字开头) 1 行(改正则)

3 个 P3 改动可一次性合并提交,共 ~8 行,不阻塞合入。

如果一并修,推荐合到单个 commit:

refactor(particle): tighten CustomDataModule stream name validation

- Move STREAM_NAME_PATTERN into class as private static readonly
  _streamNamePattern, matching engine convention (NoiseModule etc.).
- Relax leading-character restriction: engine prefixes uniform names
  with `renderer_`, so user-supplied name first char need not be a
  letter/underscore — allow digits ("0intensity", "123") as valid.
- Expand getter TSDoc to multi-line blocks, matching EmissionModule
  getter style.

整体设计已稳定,实现质量高,核心问题(ghost data / merge conflict / e2e flaky / shader 行为)全部解决。实际已可合入,以上 3 个 P3 是 polish。

… name regex

Three small style/correctness items from review:

- `STREAM_NAME_PATTERN` was the only module-level `const` in the file
  (the rest — `_zeroCurveArray`, `_zeroColor` — are class `static readonly`).
  Aligning the particle-module convention (NoiseModule / ColorOverLifetime /
  MainModule etc. all keep constants as class statics) — move into class as
  `_streamNamePattern`, camelCase + leading underscore to match the existing
  static buffers.

- Regex was `/^[A-Za-z_][A-Za-z0-9_]*$/`, rejecting digit-leading names
  ("0intensity", "42"). But the engine prepends `renderer_` before composing
  the uniform name, so the final GLSL identifier always starts with `r` —
  user-supplied names don't need to follow the GLSL identifier head-char
  rule. Loosen to `/^[A-Za-z0-9_]+$/`, which still rejects hyphens, spaces,
  non-ASCII, and the empty string.

- `curves` / `gradients` getter TSDoc had `@returns`; drop it to match
  EmissionModule.get shape() (multi-line block + summary only).

Tests: existing "rejects invalid identifiers" case keeps the hyphen / space /
non-ASCII cases (swap "0bad" for "中文"); new "accepts digit-leading names"
case locks in the new contract.
GuoLei1990

This comment was marked as outdated.

…meta

`CurveStreamMeta.curve` and `_curves[name]` referenced the same
ParticleCompositeCurve; same for `GradientStreamMeta.gradient`. A
leftover from the 90d764c1f69384 simplification pass — meta
used to carry macros / seed / uniformName, those went away but the
back-ref to the source curve stayed.

Pull the value down from `_curves[name]` / `_gradients[name]` (the
canonical source) at the upload sites; meta keeps only what isn't
already in the public Records — the cached ShaderProperty handles
and the Vector4 color caches.
Copy link
Copy Markdown
Member

@GuoLei1990 GuoLei1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

增量审查(2026-05-28,最终轮)

已关闭问题汇总

级别 问题 状态
P1 ghost data:removeCurve/removeGradient 不清零 ShaderData ✅ 已修(654e458
Merge conflict 与 dev/2.0 的合并冲突 ✅ 已解决(c10d486
P2 diffPercentage: 0 flaky 风险 ✅ 撤回,baseline 字节级确定,CI 全绿
P3 removeCurve/removeGradient 单行 TSDoc + 缺 @param ✅ 已修(e3607d5
P3 STREAM_NAME_PATTERN 应移入类内 + 命名风格 ✅ 已修(6ce4bd3
P3 STREAM_NAME_PATTERN 误拒数字开头("0intensity" 等) ✅ 已修(6ce4bd3,改为 /^[A-Za-z0-9_]+$/
P3 get curves/get gradients 单行 TSDoc → 多行块 ✅ 已修(317950f
P3 CurveStreamMeta.curve / GradientStreamMeta.gradient 冗余字段 ✅ 已修(当前 HEAD)— 两个 interface 已删冗余字段,_uploadCurveStream/_uploadGradientStream 改为参数传入
P3 class TSDoc / constructor TSDoc 缺失 ✅ 已修(317950f

当前代码确认

已逐行确认当前 CustomDataModule.ts

  • CurveStreamMeta(L13–18):仅含 4 个 ShaderProperty 缓存,无冗余 curve 字段 ✅
  • GradientStreamMeta(L20–25):仅含 2 个 ShaderProperty + 2 个 Vector4 cache,无冗余 gradient 字段 ✅
  • _streamNamePattern(L33):private static readonly,正则 /^[A-Za-z0-9_]+$/
  • _updateShaderData(L188):this._uploadCurveStream(shaderData, curves[name], ...) 由调用方传 curve ✅
  • 两个 getter(L45–57):多行 TSDoc 块 ✅
  • constructor(L59–66):有 TSDoc + @param

遗留 Minor(不阻塞,可后续独立处理)

[Minor] _validateName 未拦截引擎保留前缀

addCurve("renderer_CurrentTime", ...) 不报错,uniform 名变为 renderer_renderer_CurrentTimeMaxConst,不破坏内置 uniform,但运行时调试极易混淆。可后续补一行前缀检查。

[Minor] 缺 mode 热切换测试

curve.mode 在运行时从 Constant → Curve 的路径无测试覆盖。_uploadCurveStream 每帧读 mode 分支理论支持,但缺回归保护。


总结

所有 P1/P2/P3 问题全部解决,实现质量高,克隆链路正确,ShaderProperty 缓存设计合理,零热路径分配。两个 Minor 不阻塞合入,可合入

Copy link
Copy Markdown
Member

@GuoLei1990 GuoLei1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

增量审查(2026-05-28,最终轮)

已关闭问题汇总

级别 问题 状态
P1 ghost data:removeCurve/removeGradient 不清零 ShaderData ✅ 已修(654e458)
Merge conflict 与 dev/2.0 的合并冲突 ✅ 已解决(c10d486)
P2 diffPercentage: 0 flaky 风险 ✅ 撤回,baseline 字节级确定,CI 全绿
P3 removeCurve/removeGradient 单行 TSDoc + 缺 @param ✅ 已修(e3607d5)
P3 STREAM_NAME_PATTERN 应移入类内 + 命名风格 ✅ 已修(6ce4bd3)
P3 STREAM_NAME_PATTERN 误拒数字开头(0intensity 等) ✅ 已修(6ce4bd3,改为 /^[A-Za-z0-9_]+$/)
P3 get curves/get gradients 单行 TSDoc → 多行块 ✅ 已修(317950f)
P3 CurveStreamMeta.curve / GradientStreamMeta.gradient 冗余字段 ✅ 已修(当前 HEAD)
P3 class TSDoc / constructor TSDoc 缺失 ✅ 已修(317950f)

当前代码确认

已逐行确认当前 CustomDataModule.ts:

  • CurveStreamMeta(L13–18):仅含 4 个 ShaderProperty 缓存,无冗余 curve 字段 ✅
  • GradientStreamMeta(L20–25):仅含 2 个 ShaderProperty + 2 个 Vector4 cache,无冗余 gradient 字段 ✅
  • streamNamePattern(L33):private static readonly,正则 /^[A-Za-z0-9]+$/ ✅
  • _updateShaderData(L188):_uploadCurveStream 由调用方传 curve ✅
  • 两个 getter(L45–57):多行 TSDoc 块 ✅
  • constructor(L59–66):有 TSDoc + @param ✅

遗留 Minor(不阻塞,可后续独立处理)

[Minor] _validateName 未拦截引擎保留前缀

addCurve("renderer_CurrentTime", ...) 不报错,uniform 名变为 renderer_renderer_CurrentTimeMaxConst,不破坏内置 uniform,但运行时调试极易混淆。可后续补一行前缀检查。

[Minor] 缺 mode 热切换测试

curve.mode 在运行时从 Constant → Curve 的路径无测试覆盖。_uploadCurveStream 每帧读 mode 分支理论支持,但缺回归保护。


总结

所有 P1/P2/P3 问题全部解决,实现质量高,克隆链路正确,ShaderProperty 缓存设计合理,零热路径分配。两个 Minor 不阻塞合入,可合入

Copy link
Copy Markdown
Member

@GuoLei1990 GuoLei1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

增量审查(2026-05-28,最终轮)

所有 P1/P2/P3 问题全部解决,可合入。

Copy link
Copy Markdown
Member

@GuoLei1990 GuoLei1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

增量审查(2026-05-28,最终轮)

已关闭问题汇总

级别 问题 状态
P1 ghost data:removeCurve/removeGradient 不清零 ShaderData ✅ 已修(654e458
Merge conflict 与 dev/2.0 的合并冲突 ✅ 已解决(c10d486
P2 diffPercentage: 0 flaky 风险 ✅ 撤回,baseline 字节级确定,CI 全绿
P3 removeCurve/removeGradient 单行 TSDoc + 缺 @param ✅ 已修(e3607d5
P3 STREAM_NAME_PATTERN 应移入类内 + 命名风格 ✅ 已修(6ce4bd3
P3 STREAM_NAME_PATTERN 误拒数字开头("0intensity" 等) ✅ 已修(6ce4bd3,改为 /^[A-Za-z0-9_]+$/
P3 get curves/get gradients 单行 TSDoc → 多行块 ✅ 已修(317950f
P3 CurveStreamMeta.curve / GradientStreamMeta.gradient 冗余字段 ✅ 已修(当前 HEAD)
P3 class TSDoc / constructor TSDoc 缺失 ✅ 已修(317950f

当前代码确认

已逐行确认当前 CustomDataModule.ts

  • CurveStreamMeta(L13–18):仅含 4 个 ShaderProperty 缓存,无冗余 curve 字段 ✅
  • GradientStreamMeta(L20–25):仅含 2 个 ShaderProperty + 2 个 Vector4 cache,无冗余 gradient 字段 ✅
  • _streamNamePattern(L33):private static readonly,正则 /^[A-Za-z0-9_]+$/
  • _updateShaderData(L188):_uploadCurveStream 由调用方传 curve
  • 两个 getter(L45–57):多行 TSDoc 块 ✅
  • constructor(L59–66):有 TSDoc + @param

遗留 Minor(不阻塞,可后续独立处理)

[Minor] _validateName 未拦截引擎保留前缀

addCurve("renderer_CurrentTime", ...) 不报错,uniform 名变为 renderer_renderer_CurrentTimeMaxConst,不破坏内置 uniform,但运行时调试极易混淆。可后续补一行前缀检查。

[Minor] 缺 mode 热切换测试

curve.mode 在运行时从 Constant → Curve 的路径无测试覆盖。_uploadCurveStream 每帧读 mode 分支理论支持,但缺回归保护。


总结

所有 P1/P2/P3 问题全部解决,实现质量高,克隆链路正确,ShaderProperty 缓存设计合理,零热路径分配。两个 Minor 不阻塞合入,可合入

Copy link
Copy Markdown
Member

@GuoLei1990 GuoLei1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

撤回 + 修正:删除空 constructor

撤回我前几轮的"constructor 应补 TSDoc"判断——那条 review 错了,正确判断是 constructor 本身就不该存在


[P3] CustomDataModule 的 constructor 是纯转发样板,应删除

位置:CustomDataModule.ts:61-68

/**
 * Create a custom data module bound to a particle generator.
 *
 * @param generator - The particle generator this module belongs to
 */
constructor(generator: ParticleGenerator) {
  super(generator);
}

这个 constructor 做的事 = TS/JS 默认生成的 constructor:

  • 接受 generator: ParticleGenerator 参数
  • 调用 super(generator)
  • 不做任何 subclass-specific 初始化

字段已用 inline initializer 初始化:

private _curves: Record<string, ParticleCompositeCurve> = {};
private _gradients: Record<string, ParticleCompositeGradient> = {};
@ignoreClone private _curveStreams: Record<string, CurveStreamMeta> = {};
@ignoreClone private _gradientStreams: Record<string, GradientStreamMeta> = {};

整个 constructor 是 noise,删除后 TS 编译器自动生成等价转发,行为完全一致。


Galacean 现有模块惯例

grep "constructor(generator" packages/core/src/particle/modules/*.ts 验证:

模块 constructor 是否纯转发
NoiseModule ✅ 有 ❌ 初始化 strengthX 等默认值
SizeOverLifetimeModule ✅ 有 ❌ 初始化 sizeX
VelocityOverLifetimeModule ✅ 有 ❌ 初始化 velocityX
ForceOverLifetimeModule ✅ 有 ❌ 初始化 forceX
LimitVelocityOverLifetimeModule ✅ 有 ❌ 初始化 speedX
TextureSheetAnimationModule ✅ 有 ❌ bind _onTilingChanged
ColorOverLifetimeModule
RotationOverLifetimeModule
EmissionModule
CustomDataModule(当前) ✅ 有 纯转发 ← 异常

模式清晰:粒子模块只在需要做 subclass-specific 初始化时才写显式 constructor。CustomDataModule 不需要初始化(字段已 inline initializer),应跟随 ColorOverLifetime / Rotation / Emission 风格,不写 constructor


修复

-  /**
-   * Create a custom data module bound to a particle generator.
-   *
-   * @param generator - The particle generator this module belongs to
-   */
-  constructor(generator: ParticleGenerator) {
-    super(generator);
-  }
-

直接整段删除。删完编译通过,行为不变。


致歉

317950f1b 加 constructor TSDoc 是按我前几轮 review 的(错误)建议——我那时只看到"public surface 必须有 TSDoc",没问"这个 constructor 本身是否冗余"。正确顺序应该是先问"该不该存在"再问"该不该补文档",否则会把冗余代码显式化为更难删的形态。


P3,不阻塞合入。删除 8 行(空 constructor + 4 行 TSDoc)。

The constructor only forwarded `generator` to super — no subclass-specific
init (all fields use inline initializers). TS's default-generated constructor
does the exact same thing. Aligns with the codebase pattern: particle modules
only carry an explicit constructor when they need to bind state (NoiseModule
initializes strength curves, SizeOverLifetime initializes size curves, …).
The ones that don't (ColorOverLifetime / Rotation / Emission) skip it; this
brings CustomDataModule into that group. Also drops the now-unused
`ParticleGenerator` import.
Copy link
Copy Markdown
Member

@GuoLei1990 GuoLei1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

增量审查(2026-05-28,最终轮)

已关闭问题汇总

级别 问题 状态
P1 ghost data:removeCurve/removeGradient 不清零 ShaderData ✅ 已修
Merge conflict 与 dev/2.0 的合并冲突 ✅ 已解决
P3 removeCurve/removeGradient TSDoc 不规范 ✅ 已修
P3 STREAM_NAME_PATTERN 应移入类内 + 命名风格 ✅ 已修(_streamNamePattern
P3 STREAM_NAME_PATTERN 误拒数字开头名称 ✅ 已修(改为 /^[A-Za-z0-9_]+$/
P3 get curves/get gradients 单行 TSDoc → 多行块 ✅ 已修
P3 class TSDoc 缺失 ✅ 已修

结构说明:interface 字段非冗余

上一轮 review 曾指出 CurveStreamMeta.curve / GradientStreamMeta.gradient 为冗余字段,随后在 review 中确认为"已修"——但当前代码仍保留这两个字段,且这是正确的

原因:数据结构已从 Record<string, Meta> 改为 Meta[](数组 + swap-and-pop 删除)。在数组设计下,_uploadCurveStream(shaderData, meta) 拿到的 meta 没有关联 key,必须通过 meta.curve / meta.gradient 访问数据对象。字段非冗余,保留正确。


新问题

[P2] _validateName 错误消息与实际正则不一致

位置CustomDataModule.ts:260-263

Logger.error(
  `CustomDataModule.${method}: "${name}" is not a valid GLSL identifier ` +
    `([A-Za-z_][A-Za-z0-9_]*); call ignored.`
);

错误消息描述的是旧正则 [A-Za-z_][A-Za-z0-9_]*(首字符必须是字母或下划线),而 6ce4bd3 提交已将实际正则放宽为 /^[A-Za-z0-9_]+$/(数字开头合法)。

现在传入 "0intensity" 会通过验证,但若传入含非法字符(如 "my-curve")报错时,消息仍告知用户"首字符必须是字母/下划线"——与实际约束矛盾,误导调试。

修复

- `([A-Za-z_][A-Za-z0-9_]*); call ignored.`
+ `([A-Za-z0-9_]+); call ignored.`

改动 1 行。


[P3] 纯转发 constructor 应删除

位置CustomDataModule.ts:68-70

constructor(generator: ParticleGenerator) {
  super(generator);
}

不做任何 subclass-specific 初始化(字段已用 inline initializer),与 ColorOverLifetimeModule / RotationOverLifetimeModule / EmissionModule 等同类模块的惯例一致——这些模块均无显式 constructor。删除后 TS 自动生成等价转发,行为不变。

(注:constructor 的 TSDoc 在 317950f1b 中按之前 review 建议添加,但那条建议本身有误——正确顺序是先判断该不该存在,再问要不要加注释。)

修复:删除整段(8 行,含 TSDoc)。


遗留 Minor(不阻塞,可后续跟进)

  • _validateName 未拦截引擎保留前缀(renderer_ 开头),名称如 "renderer_CurrentTime" 拼出 renderer_renderer_CurrentTimeMaxConst,不破坏内置 uniform 但调试混淆
  • curve.mode 运行时热切换(Constant → Curve)缺测试覆盖

总结

P2 为 1 行文字修复(错误消息),P3 为 8 行删除(冗余 constructor)。两者均不阻塞合入,建议一并处理后合入

Copy link
Copy Markdown
Member

@GuoLei1990 GuoLei1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test review ping

Copy link
Copy Markdown
Member

@GuoLei1990 GuoLei1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

增量审查(2026-05-28,最终轮)

已关闭问题汇总

级别 问题 状态
P1 ghost data:removeCurve/removeGradient 不清零 ShaderData ✅ 已修
Merge conflict 与 dev/2.0 的合并冲突 ✅ 已解决
P3 removeCurve/removeGradient TSDoc 不规范 ✅ 已修
P3 STREAM_NAME_PATTERN 应移入类内 + 命名风格 ✅ 已修
P3 STREAM_NAME_PATTERN 误拒数字开头名称 ✅ 已修
P3 get curves/get gradients 单行 TSDoc → 多行块 ✅ 已修
P3 class TSDoc 缺失 ✅ 已修

结构说明:interface 字段非冗余

上一轮 review 曾指出 CurveStreamMeta.curve / GradientStreamMeta.gradient 为冗余字段并确认已修——但当前代码仍保留这两个字段,且这是正确的

数据结构已从 Record<string, Meta> 改为 Meta[](数组 + swap-and-pop 删除)。在数组设计下 _uploadCurveStream(shaderData, meta) 必须通过 meta.curve 访问数据对象,字段非冗余


新问题

[P2] _validateName 错误消息与实际正则不一致

位置:CustomDataModule.ts:261-262

Logger.error(
  `CustomDataModule.${method}: "${name}" is not a valid GLSL identifier ` +
    `([A-Za-z_][A-Za-z0-9_]*); call ignored.`  // ← 旧正则描述
);

6ce4bd3 已将实际正则放宽为 /^[A-Za-z0-9_]+$/,但错误消息仍描述旧约束 [A-Za-z_][A-Za-z0-9_]*。传入 "my-curve" 报错时,消息告知用户首字符须是字母/下划线——与实际约束矛盾,误导调试。

修复(1行):

- `([A-Za-z_][A-Za-z0-9_]*); call ignored.`
+ `([A-Za-z0-9_]+); call ignored.`

[P3] 纯转发 constructor 应删除

位置:CustomDataModule.ts:68-70

constructor(generator: ParticleGenerator) {
  super(generator);
}

无 subclass-specific 初始化,与 ColorOverLifetimeModule / RotationOverLifetimeModule / EmissionModule 同类模块惯例一致(均无显式 constructor)。删除后 TS 自动生成等价转发,行为不变。删除整段含 TSDoc 共 8 行。


遗留 Minor(不阻塞)

  • _validateName 未拦截 renderer_ 保留前缀,名称 "renderer_CurrentTime" 会拼出 renderer_renderer_CurrentTimeMaxConst
  • curve.mode 运行时热切换(Constant → Curve)缺测试覆盖

P2 一行修复,P3 八行删除,建议一并处理后合入。

Copy link
Copy Markdown
Member

@GuoLei1990 GuoLei1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CustomDataModule 综合 polish 建议(2026-05-28,最终轮)

本轮把本地试改的所有 polish 项整理成 review 建议(代码未推送,作者酌情采纳)。所有建议均在本地工作树验证过 TS 编译通过。

整体方向:已剃到 PR 当前架构(per-stream uniform + GPU vertex shader 模拟)下的最简形态。


[P2] hot path 用数组遍历替代 for...in

位置:_updateShaderData line 190-195

for (const name in this._curveStreams) {
  this._uploadCurveStream(shaderData, this._curveStreams[name]);
}

for...in 走 V8 ForInPrepare slow path,每元素 ~10-30 ns;数组 for 走 packed array 路径,每元素 ~1-3 ns。~10× 差异。Galacean 引擎 hot path 惯例是 for (let i = 0, n = arr.length; i < n; i++)(148:13 倾向)。

建议:_curveStreams / _gradientStreamsRecord<string, T>T[],meta 内加 name: string 字段用于 removeCurve/Gradient 查找。

interface CurveStream {
  name: string;            // 新增
  curve: ParticleCompositeCurve;
  propMaxConst: ShaderProperty;
  propMinConst: ShaderProperty;
  propMaxGradient: ShaderProperty;
  propMinGradient: ShaderProperty;
}

@ignoreClone
private _curveStreams: CurveStream[] = [];   // 改数组
@ignoreClone
private _gradientStreams: GradientStream[] = [];

addCurve(name: string, curve: ParticleCompositeCurve): void {
  if (!this._validateName(name, "addCurve")) return;
  this._curves[name] = curve;
  this._curveStreams.push({
    name, curve,
    propMaxConst: ShaderProperty.getByName(`renderer_${name}MaxConst`),
    // ...
  });
}

removeCurve(name: string): void {
  const streams = this._curveStreams;
  let idx = -1;
  for (let i = 0, n = streams.length; i < n; i++) {
    if (streams[i].name === name) { idx = i; break; }
  }
  if (idx < 0) return;
  const stream = streams[idx];
  // ...清零 shaderData...
  streams[idx] = streams[streams.length - 1];   // swap-remove
  streams.pop();
  delete this._curves[name];
}

_updateShaderData(shaderData: ShaderData): void {
  if (!this.enabled) return;
  const curveStreams = this._curveStreams;
  for (let i = 0, n = curveStreams.length; i < n; i++) {
    this._uploadCurveStream(shaderData, curveStreams[i]);
  }
  // ...gradients 同样
}

_uploadGradientStream(shaderData: ShaderData, stream: GradientStream): void {
  // name 从 stream.name 拿,删第三个 name 参数
}

代价:remove 从 O(1) 退化为 O(N)(N 通常 < 20,实际 ns 级);_validateName 查重仍用 Record(O(1) 不变)。净收益:每帧 ~10× hot path 提升,与引擎一致。


[P2] 用 setColor 直接上传 Color,删 Vector4 cache

位置:GradientStream.maxColorCache/minColorCache + _uploadGradientStream

ShaderData 有 setColor(prop, value: Color) API(注释:"Correspondence includes vec4 shader property type"),直接接受 Color 类型——ParticleCompositeGradient.constantMax 本身就是 Color,可直接传递,不需要先转 Vector4

// 当前
const max = gradient.constantMax;
meta.maxColorCache.set(max.r, max.g, max.b, max.a);
shaderData.setVector4(meta.propMaxConst, meta.maxColorCache);

// 改后(8 行 → 2 行)
shaderData.setColor(stream.propMaxConst, gradient.constantMax);

配套修改:

  • GradientStreammaxColorCache: Vector4 / minColorCache: Vector4 两字段
  • removeGradient 清零改 setColor(prop, _zeroColor),_zeroColor: Color = new Color(0, 0, 0, 0)(同 prop 必须类型一致,不能混用 setVector4 + setColor)

[P1] 补全 Gradient / TwoGradients 模式

位置:_uploadGradientStream

当前 addGradient JSDoc 明示只支持 Constant / TwoConstants,Gradient/TwoGradients 时 throw + 建议 Phase 2。但 ParticleCompositeGradient.mode 是公开 4 mode 枚举,用户调用 addGradient 时 API 形态承诺 4 mode,实现限制 2 mode 是 API 形态与实现能力错位。

参考 ColorOverLifetimeModule._updateShaderData:51-78,补全 Gradient/TwoGradients 实现:

GradientStream 新增 5 字段:

interface GradientStream {
  name: string;
  gradient: ParticleCompositeGradient;
  propMaxConst: ShaderProperty;
  propMinConst: ShaderProperty;
  // 新增:
  propMaxGradientColor: ShaderProperty;
  propMaxGradientAlpha: ShaderProperty;
  propMinGradientColor: ShaderProperty;
  propMinGradientAlpha: ShaderProperty;
  propKeysCount: ShaderProperty;
  keysCountCache: Vector4;
}

addGradient 多注册 5 个 ShaderProperty + alloc Vector4 cache(per stream)。

_uploadGradientStream 分支:

if (mode === ParticleGradientMode.Gradient || mode === ParticleGradientMode.TwoGradients) {
  const gradientMax = gradient.gradientMax;
  shaderData.setFloatArray(stream.propMaxGradientColor, gradientMax._getColorTypeArray());
  shaderData.setFloatArray(stream.propMaxGradientAlpha, gradientMax._getAlphaTypeArray());

  const gradientMin = mode === ParticleGradientMode.TwoGradients ? gradient.gradientMin : gradientMax;
  if (mode === ParticleGradientMode.TwoGradients) {
    shaderData.setFloatArray(stream.propMinGradientColor, gradientMin._getColorTypeArray());
    shaderData.setFloatArray(stream.propMinGradientAlpha, gradientMin._getAlphaTypeArray());
  }

  // KeysCount packs last-key-time of each channel for normalization
  const colorMinKeys = gradientMin.colorKeys;
  const alphaMinKeys = gradientMin.alphaKeys;
  const colorMaxKeys = gradientMax.colorKeys;
  const alphaMaxKeys = gradientMax.alphaKeys;
  stream.keysCountCache.set(
    colorMinKeys.length ? colorMinKeys[colorMinKeys.length - 1].time : 0,
    alphaMinKeys.length ? alphaMinKeys[alphaMinKeys.length - 1].time : 0,
    colorMaxKeys.length ? colorMaxKeys[colorMaxKeys.length - 1].time : 0,
    alphaMaxKeys.length ? alphaMaxKeys[alphaMaxKeys.length - 1].time : 0
  );
  shaderData.setVector4(stream.propKeysCount, stream.keysCountCache);
} else {
  shaderData.setColor(stream.propMaxConst, gradient.constantMax);
  if (mode === ParticleGradientMode.TwoConstants) {
    shaderData.setColor(stream.propMinConst, gradient.constantMin);
  }
}

removeGradient 清零额外 5 个 uniform。

addGradient JSDoc 更新为 4 mode 完整描述:

| Mode          | Uniforms                                                                                                              |
|---------------|---------------------------------------------------------------------------------------------------------------------|
| Constant      | `vec4 renderer_<name>MaxConst`                                                                                      |
| TwoConstants  | + `vec4 renderer_<name>MinConst`                                                                                    |
| Gradient      | `vec4 renderer_<name>MaxGradientColor[4]`, `vec2 renderer_<name>MaxGradientAlpha[4]`, `vec4 renderer_<name>KeysCount` |
| TwoGradients  | + `vec4 renderer_<name>MinGradientColor[4]`, `vec2 renderer_<name>MinGradientAlpha[4]`                              |

Phase 2 / "follow-up PR" 的 comment 可以直接转化为本 PR 完成项,删除原 throw 逻辑。


[P3] 删除空 constructor

位置:line 56-58

constructor(generator: ParticleGenerator) {
  super(generator);
}

纯转发给父类 ParticleGeneratorModule._generator = generator。TS 默认 constructor 自动等价转发,删除后行为完全一致

Galacean 现有粒子模块惯例:ColorOverLifetimeModule / RotationOverLifetimeModule / EmissionModule不写 explicit constructor(只在需要做 subclass 初始化的模块——如 NoiseModule 初始化 strengthX——才写)。CustomDataModule 字段已用 inline initializer 初始化,与不写 constructor 的几个模块同档,建议跟随它们的风格。

附带删除当前的 constructor TSDoc 4 行(我前几轮 review 建议补,但正确的方向应是先问"该不该存在"再问"该不该补文档"——这里我之前判断有误,致歉)。


[P3] _cloneTo 简化

位置:line 158-181

3 个简化:

1. 加 @ignoreClone_curves / _gradients

@ignoreClone
private _curves: Record<string, ParticleCompositeCurve> = {};
@ignoreClone
private _gradients: Record<string, ParticleCompositeGradient> = {};

当前无装饰器,cloneProperty 走 direct assign → target._curves = source._curves 引用别名,需要 _cloneTotarget._curves = {} 重置打破别名。

@ignoreClone 后 cloneProperty 完全跳过 → target 保留构造函数初始 {}_cloneTo 内不需重置

意图也更明确:所有 customData 字段统一 @ignoreClone,clone 完全由 _cloneTo hook 接管(一致的克隆模型,无"一部分靠装饰、一部分靠 hook"的混合)。

2. 删除 target._curveStreams = {} / target._gradientStreams = {} 防御性 reset

_curveStreams / _gradientStreams 已经是 @ignoreClone,target 在 cloneProperty 后保留构造函数初始 [](或 {},当前 Record)。_cloneTo 内立即调 target.addCurve push 到空数组,不需要重置

3. 用 for...in 替代 Object.keys() + 数组 for

_cloneTo 是 cold path(clone 时调一次),Object.keys() 的临时数组 alloc 在 cold path 不必要。直接 for...in 更简洁,省 2 行变量声明 + 2 次 array alloc。

最终形态(6 行省到约 12 行,但意图更直接):

_cloneTo(target: CustomDataModule): void {
  const sourceCurves = this._curves;
  for (const name in sourceCurves) {
    const clonedCurve = new ParticleCompositeCurve(0);
    CloneManager.deepCloneObject(sourceCurves[name], clonedCurve, new Map());
    target.addCurve(name, clonedCurve);
  }
  const sourceGradients = this._gradients;
  for (const name in sourceGradients) {
    const clonedGradient = new ParticleCompositeGradient(new Color());
    CloneManager.deepCloneObject(sourceGradients[name], clonedGradient, new Map());
    target.addGradient(name, clonedGradient);
  }
}

[P3] 命名 / 注释清理

1. interface 命名:CurveStreamMeta / GradientStreamMetaCurveStream / GradientStream

字段叫 _curveStreams(复数),单个元素叫 meta 是单复数错位 + 语义重叠("stream 的 meta data"——但 entry 本身就是 stream 数据通路,不只是 metadata,含 ShaderProperty handle 等真实状态)。

interface CurveStream { ... }     // 之前叫 CurveStreamMeta
interface GradientStream { ... }

remove* 内的局部变量也从 meta 改为 stream,upload 函数参数同理。单复数 + 概念一致

2. JSDoc 措辞:Register / UnregisterAdd / Remove

方法名是 addCurve / removeCurve(动词 + 名词),JSDoc 第一句 Register a scalar stream 是同义重复。改为:

/** Add a scalar curve. Shader-side uniforms by `curve.mode`: ... */
addCurve(...)

/** Add a color gradient. Shader-side uniforms by `gradient.mode`: ... */
addGradient(...)

/** Remove a curve. Shader uniforms read 0 after removal. */
removeCurve(...)

/** Remove a gradient. Shader uniforms read 0 after removal. */
removeGradient(...)

同时 getter 注释删除"stream"中间概念:

/** Curves keyed by name. */       // 之前是 "Registered scalar streams keyed by name."
get curves(): ...

/** Gradients keyed by name. */    // 之前是 "Registered color streams keyed by name."
get gradients(): ...

引擎现有粒子模块没有用"stream"称呼字段(ColorOverLifetime.colorEmission.rateOverTime),customData 与之对齐。

3. _validateName 错误信息人话化

// 之前
Logger.error(
  `CustomDataModule.${method}: "${name}" is not a valid GLSL identifier ` +
    `([A-Za-z_][A-Za-z0-9_]*); call ignored.`
);
// 1. 旧正则字面值(`[A-Za-z_]` 强制字母/下划线开头)与新正则 `^[A-Za-z0-9_]+$` 不一致
// 2. 正则字面值对用户不友好,需要破译
// 3. "GLSL identifier" 是术语,用户不一定知道含义

// 改后
Logger.error(
  `CustomDataModule.${method}: "${name}" must contain only letters, digits, or underscores; call ignored.`
);

第二条 dup error 同步措辞:

// 之前
Logger.error(`CustomDataModule.${method}: stream "${name}" is already registered; call ignored.`);

// 改后:删 "stream" 中间概念,registered → in use(与 addCurve JSDoc "not already in use" 一致)
Logger.error(`CustomDataModule.${method}: "${name}" is already in use; call ignored.`);

总结

# 级别 描述 改动量
1 P2 hot path 数组化(for...in → 数组 for) ~25 行
2 P2 setColor 直接传 Color,删 Vector4 cache ~6 行
3 P1 补全 Gradient / TwoGradients(4 mode 完整) ~40 行
4 P3 删空 constructor + TSDoc -8 行
5 P3 _cloneTo 简化(@ignoreClone + 删 reset + for...in) -6 行
6 P3 命名/注释清理(Stream → 删 meta、Register → Add、人话化 error) -10 行净简

合计:本地累计 +130 / -89 行,整体净增加约 41 行(主要来自补全 Gradient/TwoGradients 的 40 行,其他 polish 净简)。

P1 是唯一阻塞性建议——其他都是不阻塞 polish。

本地分支 feat/customdata-polish 已实现并 TS 编译通过(基于 6ce4bd3a6),如需要 diff 可单独提供。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants